Add table namespace classifier #761
Conversation
ming535
force-pushed
the
feature-namespace-classifier
branch
from
766053b
to
f4451d8
Compare
| "github.com/juju/errors" | ||
| ) | ||
|
|
||
| var tablePrefix = []byte{'t'} |
There was a problem hiding this comment.
seem we must support index prefix later too.
| @@ -0,0 +1,46 @@ | |||
| package server | |||
There was a problem hiding this comment.
use table_namespace_classifier.go , we don't support CamelCase for the file name.
There was a problem hiding this comment.
Also please add license header.
|
Please add test |
| for name, ns := range classifier.nsInfo.namespaces { | ||
| startTable := core.DecodeTableID(regionInfo.StartKey) | ||
| endTable := core.DecodeTableID(regionInfo.EndKey) | ||
| for _, tableID := range ns.TableIDs { |
There was a problem hiding this comment.
if we have many tables in one namespace, seem this range check will hurt the performance.
Maybe is it better to use a hash map for TableIDs ?
There was a problem hiding this comment.
I think it's ok. Because the number of regions may be a lot, now we always try the best to avoid traversing regions.
| @@ -0,0 +1,46 @@ | |||
| package server | |||
There was a problem hiding this comment.
Also please add license header.
| ) | ||
|
|
||
| type tableNamespaceClassifier struct { | ||
| nsInfo *namespacesInfo |
There was a problem hiding this comment.
Where is namespaceInfo defined? Forget to submit?
There was a problem hiding this comment.
It's in another PR #747
Saving/Loading namespace and classifier are separated.
There was a problem hiding this comment.
This PR is supposed to merge into dantin/namespace
| for name, ns := range classifier.nsInfo.namespaces { | ||
| startTable := core.DecodeTableID(regionInfo.StartKey) | ||
| endTable := core.DecodeTableID(regionInfo.EndKey) | ||
| for _, tableID := range ns.TableIDs { |
There was a problem hiding this comment.
I think it's ok. Because the number of regions may be a lot, now we always try the best to avoid traversing regions.
|
@siddontang @disksing merged this PR to dantin/namespace and will add tests in dantin/namespace, so that you can review all the changes in this PR: #747 |
Add table namespace classifier